Skip to content

Conversation

@miyazakh
Copy link
Contributor

Fixes issues on scan-build LLVM-19

scan-build --status-bugs make -C test/ 
scan-build --status-bugs make -C benchmark/ 
scan-build --status-bugs make -C examples/ 
scan-build --status-bugs make -C tools/whnvmtool/

There is one issue, which is warning: Value stored to 'ret' is never read in wolfcrypt\src\random.c against tools/whnvmtool/, remaining. That will be separately fixed.

Github action addition for changes in the future will be submitted as separate PR.

@miyazakh miyazakh assigned miyazakh and wolfSSL-Bot and unassigned miyazakh Sep 17, 2025
Copy link
Contributor

@JacobBarthelmeh JacobBarthelmeh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks great. Only thing that stood out to me is the loss of capturing return values. I think we should act on the return values if scan-build is complaining about them not being used, or the internal function should be changed to a void return if no return values are expected.

@bigbrett bigbrett assigned miyazakh and unassigned wolfSSL-Bot Sep 23, 2025
@bigbrett
Copy link
Contributor

@miyazakh looks like there are merge conflicts. Can you please address my comments as well as resolve them.

I also would like to see scan-build integrated as a GH action

@miyazakh
Copy link
Contributor Author

clang-tidy starts complaining to wh_server_keystore.c even though the PR doesn't touch it.

@bigbrett
Copy link
Contributor

clang-tidy starts complaining to wh_server_keystore.c even though the PR doesn't touch it.

@miyazakh Should be fixed if you rebase on main, sorry about that!

Copy link
Contributor

@billphipps billphipps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! A few minor suggestions and a couple of questions. This should be merged prior to #158.

@miyazakh miyazakh assigned bigbrett and unassigned miyazakh Sep 27, 2025
Copy link
Contributor

@billphipps billphipps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of excellent fixes and finds! Thank you!

I recommend we ignore some of the error returns as in the original code. Let me know if you agree or if we should find an alternate path on how to handle secondary errors.

I think exiting early during reading the NVM directory objects is an error and must be corrected.

@bigbrett bigbrett self-requested a review October 2, 2025 14:34
@bigbrett
Copy link
Contributor

bigbrett commented Oct 2, 2025

@billphipps please merge when you are ok with it

Copy link
Contributor

@billphipps billphipps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks!

@billphipps billphipps merged commit 1eda4bc into wolfSSL:main Oct 2, 2025
10 checks passed
@miyazakh miyazakh deleted the scan_build_issues branch October 2, 2025 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants